Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Theme] configure appearance color mode #203406

Merged
merged 43 commits into from
Dec 30, 2024
Merged

[Theme] configure appearance color mode #203406

merged 43 commits into from
Dec 30, 2024

Conversation

sebelga
Copy link
Contributor

@sebelga sebelga commented Dec 9, 2024 •
edited by kibanamachine
Loading

In this PR I've updated the UI to set the appearance color mode in both stateful and Cloud/serverless.

How to test

On prem

  • Launch Kibana
  • Navigate to the user profile
  • You should see the 4 options to set the color mode.

Cloud

  • Add the following to your kibana.dev.yml
xpack.cloud.id: "ftr_fake_cloud_id:aGVsbG8uY29tOjQ0MyRFUzEyM2FiYyRrYm4xMjNhYmM="
  • Make the following change on L29 of the maybe_add_cloud_links.ts file
--- a/x-pack/plugins/cloud_integrations/cloud_links/public/maybe_add_cloud_links/maybe_add_cloud_links.ts
+++ b/x-pack/plugins/cloud_integrations/cloud_links/public/maybe_add_cloud_links/maybe_add_cloud_links.ts
@@ -26,6 +26,7 @@ export function maybeAddCloudLinks({ core, security, cloud }: MaybeAddCloudLinks
   const userObservable = defer(() => security.authc.getCurrentUser()).pipe(
     // Check if user is a cloud user.
     map((user) => user.elastic_cloud_user),
+    map(() => true),
     // If user is not defined due to an unexpected error, then fail *open*.
     catchError(() => of(true)),
     filter((isElasticCloudUser) => isElasticCloudUser === true),
  • You should now see the "Appearance" link in the user menu

Release note

We have improved the UI to set the color mode in Cloud and Serverless. Allowing the "System" value which will sync the dark mode with the system value.

Screenshots

On prem

Screenshot 2024-12-16 at 10 46 35 Screenshot 2024-12-16 at 10 46 43

Cloud

Screenshot 2024-12-16 at 10 45 03 Screenshot 2024-12-16 at 10 46 08 Screenshot 2024-12-16 at 10 46 16

Serverless

Screenshot 2024-12-16 at 16 43 56

Fixes https://github.com/elastic/kibana-team/issues/1325.

@sebelga sebelga self-assigned this Dec 9, 2024
@sebelga sebelga added Feature:Security/User Profile backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) labels Dec 9, 2024
@sebelga
Copy link
Contributor Author

sebelga commented Dec 9, 2024

/ci

3 similar comments
@sebelga
Copy link
Contributor Author

sebelga commented Dec 9, 2024

/ci

@sebelga
Copy link
Contributor Author

sebelga commented Dec 9, 2024

/ci

@sebelga
Copy link
Contributor Author

sebelga commented Dec 11, 2024

/ci

@sebelga sebelga force-pushed the dark-mode/appearance-selector branch from c6f0de1 to 46785d4 Compare December 11, 2024 17:41
@sebelga
Copy link
Contributor Author

sebelga commented Dec 11, 2024

/ci

1 similar comment
@sebelga
Copy link
Contributor Author

sebelga commented Dec 12, 2024

/ci

@sebelga sebelga changed the title [Theme] configure appearance and contrast [Theme] configure appearance color mode Dec 12, 2024
@sebelga sebelga force-pushed the dark-mode/appearance-selector branch from 1ae051c to 4f4ce28 Compare December 13, 2024 12:58
@sebelga
Copy link
Contributor Author

sebelga commented Dec 18, 2024

Thanks for the review @eokoneyo !

I see, that could be a nice enhancement, could we defer it to a separate PR? I am not sure yet where that code would live, maybe close to the rendering or theme core services. As the core team already approved this PR, I'd rather add that change separately.

The current behaviour is what we have today with the ui settings so I guess our users are used to refresh the page if they tweak their system color mode setting (I guess that doesn't happen too often 😊 )

azasypkin
azasypkin approved these changes Dec 19, 2024
View reviewed changes
Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from a security perspective. From a user perspective, I wish there was less white space in the Appearance modal, but I’ll defer that to you, the Shared UX team, and the design team. 🙂

@sebelga
Copy link
Contributor Author

sebelga commented Dec 20, 2024

Thanks for the review @azasypkin!

From a user perspective, I wish there was less white space in the Appearance modal

Yeah it's a compromise to not truncate the the warning callout title. I did reduce 20px width in dc6273e

@eokoneyo I added a listener for when the system color mode changes and render a toast message to invite the user to reload the page. Could you have another look? thanks!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
cloudLinks 90 92 +2
core 422 427 +5
total +7

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/core-theme-browser-internal 0 1 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
security 541.4KB 542.3KB +917.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
cloudLinks 31.0KB 34.3KB +3.3KB
core 447.7KB 449.5KB +1.8KB
security 69.1KB 69.2KB +95.0B
total +5.2KB
Unknown metric groups

API count

id before after diff
@kbn/core-theme-browser-internal 0 1 +1

History

cc @sebelga

eokoneyo
eokoneyo approved these changes Dec 30, 2024
View reviewed changes
Copy link
Contributor

@eokoneyo eokoneyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the changes in on Prem and Serverless, when appearance is set to system and a color mode event occurs a toast is presented to the user. Thanks @sebelga for looking further into this ❤️

@sebelga sebelga merged commit 20d3700 into elastic:main Dec 30, 2024
8 checks passed
@sebelga sebelga deleted the dark-mode/appearance-selector branch December 30, 2024 09:43
@sebelga
Copy link
Contributor Author

sebelga commented Dec 30, 2024

Thanks for the review @eokoneyo ! 👍

@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12544770392

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 30, 2024
@sebelga
[Theme] configure appearance color mode (elastic#203406)
10807c5 
(cherry picked from commit 20d3700)
@kibanamachine kibanamachine mentioned this pull request Dec 30, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 30, 2024
@kibanamachine @sebelga
[8.x] [Theme] configure appearance color mode (#203406) (#205239)
70b1bee 
# Backport

This will backport the following commits from `main` to `8.x`:
- [[Theme] configure appearance color mode
(#203406)](#203406)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Sébastien
Loix","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-30T09:43:48Z","message":"[Theme]
configure appearance color mode
(#203406)","sha":"20d37000ef1e8014199a44b7b4a1e308dd4c2e52","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","v9.0.0","Team:SharedUX","Feature:Security/User
Profile","backport:prev-minor"],"title":"[Theme] configure appearance
color
mode","number":203406,"url":"https://github.com/elastic/kibana/pull/203406","mergeCommit":{"message":"[Theme]
configure appearance color mode
(#203406)","sha":"20d37000ef1e8014199a44b7b4a1e308dd4c2e52"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203406","number":203406,"mergeCommit":{"message":"[Theme]
configure appearance color mode
(#203406)","sha":"20d37000ef1e8014199a44b7b4a1e308dd4c2e52"}}]}]
BACKPORT-->

Co-authored-by: Sébastien Loix <[email protected]>
stratoula pushed a commit to stratoula/kibana that referenced this pull request Jan 2, 2025
@sebelga @stratoula
[Theme] configure appearance color mode (elastic#203406)
39b019b
benakansara pushed a commit to benakansara/kibana that referenced this pull request Jan 2, 2025
@sebelga @benakansara
[Theme] configure appearance color mode (elastic#203406)
e94ee20
cqliu1 pushed a commit to cqliu1/kibana that referenced this pull request Jan 2, 2025
@sebelga @cqliu1
[Theme] configure appearance color mode (elastic#203406)
4b56c7a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukeelmers lukeelmers lukeelmers left review comments

@jloleysens jloleysens jloleysens approved these changes

@eokoneyo eokoneyo eokoneyo approved these changes

@azasypkin azasypkin azasypkin approved these changes

Assignees

@sebelga sebelga

Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Security/User Profile release_note:enhancement Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.18.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@sebelga @elasticmachine @petrklapka @kibanamachine @JasonStoltz @eokoneyo @lukeelmers @azasypkin @jloleysens